[codex] Structure Bitbucket API errors#3457
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Effect service review: one convention issue, recurring across six new error classes in apps/server/src/sourceControl/BitbucketApi.ts.
The split from the generic BitbucketApiError into tagged classes is the right direction. The problem is that six of the new classes encode the same distinction twice — a specific error tag plus a single-value operation literal — which the conventions disallow. The fix is to drop the redundant operation field from those classes (inlining the constant operation name in each message getter) while keeping operation only on the shared multi-operation errors (BitbucketRequestError, BitbucketResponseError, BitbucketResponseBodyReadError, BitbucketResponseDecodeError).
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Approved Refactor that replaces a generic BitbucketApiError with multiple specific error classes for better error handling granularity. Changes are internal to error structure with comprehensive test coverage and include a security improvement by excluding response bodies from error messages. You can customize Macroscope's approvability policy. Learn more. |
bb38a29 to
ecba319
Compare
Dismissing prior approval to re-evaluate ecba319
ecba319 to
bbb7b63
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Raw response body in message
- Added a 512-character truncation limit to the HTTP response body in the responseError() function before storing it in BitbucketResponseError, preventing sensitive data exposure and excessively large error payloads.
Or push these changes by commenting:
@cursor push 482367cda8
Preview (482367cda8)
diff --git a/apps/server/src/sourceControl/BitbucketApi.ts b/apps/server/src/sourceControl/BitbucketApi.ts
--- a/apps/server/src/sourceControl/BitbucketApi.ts
+++ b/apps/server/src/sourceControl/BitbucketApi.ts
@@ -27,6 +27,8 @@
const DEFAULT_API_BASE_URL = "https://api.bitbucket.org/2.0";
+const MAX_RESPONSE_BODY_LENGTH = 512;
+
const BitbucketApiEnvConfig = Config.all({
baseUrl: Config.string("T3CODE_BITBUCKET_API_BASE_URL").pipe(
Config.withDefault(DEFAULT_API_BASE_URL),
@@ -488,15 +490,20 @@
cause,
}),
),
- Effect.flatMap((body) =>
- Effect.fail(
+ Effect.flatMap((body) => {
+ const trimmed = body.trim();
+ const bounded =
+ trimmed.length > MAX_RESPONSE_BODY_LENGTH
+ ? `${trimmed.slice(0, MAX_RESPONSE_BODY_LENGTH)}…`
+ : trimmed;
+ return Effect.fail(
new BitbucketResponseError({
operation,
status: response.status,
- responseBody: body,
+ responseBody: bounded,
}),
- ),
- ),
+ );
+ }),
);
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit bbb7b63. Configure here.
bbb7b63 to
b6efbe9
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
b6efbe9 to
d0ebe0d
Compare
Dismissing prior approval to re-evaluate d0ebe0d
1 participant
Summary
BitbucketApiError.detailbucket with distinct Schema error classes for transport, HTTP response, body-read, decode, repository resolution, locator, body-file, and checkout failuresSchema.ispredicate so checkout preserves already-structured Bitbucket failures by tagValidation
vp test run apps/server/src/sourceControl/BitbucketApi.test.ts apps/server/src/sourceControl/BitbucketSourceControlProvider.test.tsvp check(passes with 20 pre-existing warnings)vp run typecheckNote
Medium Risk
Changes error shapes and HTTP failure messaging (no embedded API bodies); callers matching on
detailor the old single class need updates, but user-facing messages stay stable and leakage risk is reduced.Overview
Replaces the monolithic
BitbucketApiError(with free-formdetail) with tagged Schema error classes and aBitbucketApiErrorunion plusisBitbucketApiErrorfor narrowing.Failures are split by kind: transport (
BitbucketRequestError), HTTP status (BitbucketResponseErrorwithresponseBodyLengthonly—no response body text in messages or fields), body read (BitbucketResponseBodyReadError), JSON decode, repository resolution/locator, PR body file read, and checkout (BitbucketCheckoutErrorwithcwd/reference).checkoutPullRequestre-throws existing union members viaisBitbucketApiErrorinstead of re-wrapping.Tests assert specific error classes, redaction of secrets from diagnostics, body-read cause chains, and provider-layer cause preservation without leaking upstream messages.
Reviewed by Cursor Bugbot for commit d0ebe0d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace generic Bitbucket API errors with typed error classes
BitbucketRequestError,BitbucketResponseError,BitbucketResponseBodyReadError,BitbucketResponseDecodeError,BitbucketCheckoutError,BitbucketRepositoryLocatorError, etc.) replacing the previous singleBitbucketApiErrorclass in BitbucketApi.ts.operation,status,responseBodyLength,cwd,reference) rather than freeformdetailstrings.BitbucketApiErroris now aSchema.Unionof all specific error types with an exportedisBitbucketApiErrorpredicate.BitbucketApiErrorwithdetail/operationfields must migrate to the new specific error types and their respective fields.Macroscope summarized d0ebe0d.